Use fewer connections in the multi-version client (release-6.3) #4677
Conversation
…only activates the client library that can connect.
… terminate the database connection.
…I versions < 610). Fix reference counting of DLDatabase objects to avoid leaking the underlying database handle. Update release notes to note that clients older than 6.2 still create extra connections.
… the version async var
…ld versions. Update the C bindings implementation of get_server_protocol to convert the ProtocolVersion object into a uint64_t. Rename a misleading protocol version alias.
Use nullptr instead of NULL Use const& for a parameter Add some comments
… around quickly closing the database.
…termine the protocol version in order to trigger leader monitoring communication.
…ed. Avoid using any state when cancelled. Fix race between setting up the protocol version monitor and destroying the database.
… in lambdas, instead capturing a full Reference.
|
Note that the primary differences with the master branch are:
|
| @@ -737,6 +737,13 @@ ACTOR Future<Void> connectionKeeper(Reference<Peer> self, | |||
|
|
|||
| conn->close(); | |||
| conn = Reference<IConnection>(); | |||
|
|
|||
| // Old versions will throw this error, and we don't want to forget their protocol versions. | |||
There was a problem hiding this comment.
If I understood correctly it is supposed to be old version will not throw this error, and will hang up? https://github.com/apple/foundationdb/blob/master/fdbrpc/FlowTransport.actor.cpp#L1224
There was a problem hiding this comment.
What happens depends on what side of the connection you are on. If you are sending the connect packet, the connection will drop and you won't learn anything about the protocol version.
If you are receiving the connect packet, then it will hang up on the old connection (i.e. not send anything), but locally it will throw an error that cleans up the connection state. We do, however, learn what the protocol version is in this case, and we're choosing not to discard it by ignoring this error.
There was a problem hiding this comment.
This differs from the newer versions, by the way, in the fact that new versions leave the connection open but don't exchange any real messages over it. The idea is that by leaving the connection open, we can cheaply keep track of the fact that the two sides are not compatible.
sfc-gh-abeamon
deleted the
feature-mvc-monitor-protocol-version-release-6.3
branch
This is a backport of #4667 and #4733, with various parts that aren't applicable removed. This is being backported to support upgrade scenarios with 6.3 that require lowering the cost of the multi-version client with two or more external clients.
This updates the multi-version client to monitor the protocol version of the cluster and use only the primary and one appropriately-versioned external client to connect.
As a result, this should eliminate the impact of incompatible clients on the cluster.
The following real-world tests were performed to verify that the client functioned and that we had the expected number of connections:
Code-Reviewer Section
The general guidelines can be found here.
Please check each of the following things and check all boxes before accepting a PR.
For Release-Branches
If this PR is made against a release-branch, please also check the following:
release-branchormasterif this is the youngest branch)